-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add triggerable exit flow #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
feat: add locator into WithdrawalVault
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the significant progress here 👍
Looked briefly this time 👀 Will get back a bit later
contracts/0.8.9/WithdrawalVault.sol
Outdated
receive() external payable {} | ||
|
||
function forcedExit(bytes[] calldata pubkeys, address sender) external payable { | ||
if (msg.sender != address(getLocator().validatorsExitBusOracle())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we don't allow this method to be called by the DAO directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, DAO should send the report hash first for 2 reasons:
- exit more than 600 per/report
- depends on the
exitFee
- can reverts early
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second pass has been done 👀
contracts/0.8.9/WithdrawalVault.sol
Outdated
uint256 vaultBalance = address(this).balance - msg.value; | ||
uint256 fee = msg.value; | ||
|
||
uint256 keysCount = pubkeys.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should check if non zero and isn't too large both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we check it in VEBO contract here
_saveReportData(reportHash, requestsCount); | ||
} | ||
|
||
function forcedExitPubkeys(bytes[] calldata keys, ReportData calldata data) external payable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no auth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes - its a permissionless part
Co-authored-by: Eugene Mamin <[email protected]>
Code Coverage Summary
Diff against master
Results for commit: 8a77a87 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Closed as ported to lidofinance/lido-dao#859 |
No description provided.